feat(branch-keys): add hv-2 spec#297
Conversation
texastony
left a comment
texastony
left a comment
There was a problem hiding this comment.
Some nits, need to do a proper review when I get home.
| If the `hierarchy-version` is v1, AWS KMS encryption context MUST be same as [branch key context](#branch-key-context). | ||
| If the `hierarchy-version` is v2, AWS KMS encryption context MUST be the [encryption context](../structures.md#encryption-context) send by users without any transformation. | ||
|
|
||
| AWS KMS encryption context MUST be always the same encryption context send by user regardless of any variation of encryption context (i.e.: ACTIVE Encryption Context, DECRYPT_ONLY Encryption Context, Beacon Key Encryption Context and Custom Encryption Context). |
There was a problem hiding this comment.
This line reads funny. I think I understand what we are trying to say.
But I think we need to clean it up.
There was a problem hiding this comment.
I rephrase it to
"""
If the hierarchy-version is v2, the encryption context send to AWS KMS MUST always match the encryption context originally sent by the user, without any variations. This requirement applies regardless of the type of encryption context being used, whether it is the ACTIVE encryption context, the DECRYPT_ONLY encryption context, the Beacon Key encryption context, or a custom encryption context.
"""
lmk if you have any suggestion
Co-authored-by: Tony Knapp <5892063+texastony@users.noreply.github.com>
|
|
||
| # 1 Plain-text Commitment instead of KMS Encryption Context | ||
|
|
||
| The KMS RSA Keyring has already solved the need |
There was a problem hiding this comment.
nit: the ecdh keyrings as well
| A root key used to then derive different beacon keys per beacon. | ||
| - [UUID](https://www.ietf.org/rfc/rfc4122.txt): a universally unique identifier that can be represented as a byte sequence or a string. | ||
|
|
||
| #### kms-arn |
There was a problem hiding this comment.
another nit:
i think kms-arn and hierachy version are meant to be bullet points under definitions and not their own sections?
There was a problem hiding this comment.
I like having them as sections so we can link to them.
There was a problem hiding this comment.
fair, it's just hard to read
| These are any members of the Branch Key's Context whose key is not prefixed by `aws-crypto-ec:` OR another reserved Branch Key Context Key Name, such as `kms-arn` or `hierarchy-version`. | ||
| Such Key-Value pairs were NOT added to a Branch Key Item by a Crypto Tools product. |
There was a problem hiding this comment.
How are revered keys not added by CryptoTools?
There was a problem hiding this comment.
NOT (prefixed with aws-crypto-ec: or another reserved word).
There was a problem hiding this comment.
So the NOT applies to both sides of the OR.
| - [KMS ARN](#kms-arn) | ||
| - [Create Time](#create-time) | ||
| - [Hierarchy Version](#hierarchy-version) |
There was a problem hiding this comment.
This is breaking. We may consider this lightly breaking because who else is constructing these. Just bringing it up.
There was a problem hiding this comment.
Fair. I think it is worth it.
I have a ticket right now where this would solve an issue.
Co-authored-by: Tony Knapp <5892063+texastony@users.noreply.github.com>
Co-authored-by: Lucas McDonald <lucasmcdonald3@gmail.com>
…_context_key_values.md Co-authored-by: Tony Knapp <5892063+texastony@users.noreply.github.com>
Co-authored-by: Tony Knapp <5892063+texastony@users.noreply.github.com>
…thout_kms_ec.md Co-authored-by: José Corella <39066999+josecorella@users.noreply.github.com>
|
Review |
8 participants
Issue #, if available:
Description of changes:
Crypto tools is adding a new feature (Hierarchical Keyring V2) in MPL main branch. This PR adds the specification for it.
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.
Check any applicable: